Skip to content

feat(server): adapt Hubble 2.0 frontend APIs and implement default graph/role management#3008

Open
Yeaury wants to merge 23 commits into
apache:masterfrom
Yeaury:feat/hubble-api-compatibility
Open

feat(server): adapt Hubble 2.0 frontend APIs and implement default graph/role management#3008
Yeaury wants to merge 23 commits into
apache:masterfrom
Yeaury:feat/hubble-api-compatibility

Conversation

@Yeaury
Copy link
Copy Markdown

@Yeaury Yeaury commented Apr 24, 2026

Purpose of the PR

Adapt API endpoints required by the Hubble 2.0 frontend to achieve feature parity with the internal version. The community edition of HugeGraph Server currently lacks several critical APIs that the Hubble frontend depends on (graph profile listing, default graph management, default role management, schema templates, etc.), preventing the Hubble frontend from properly using core graph and role management features. This PR adds the missing APIs to enable full integration between the Hubble 2.0 frontend and the community edition Server.

Link apache/hugegraph-toolchain#632

Main Changes

1. GraphsAPI — Graph Management Endpoint Enhancements (GraphsAPI.java)

  • GET /graphspaces/{graphspace}/graphs/profile — New graph profile listing endpoint with prefix filtering, default-graph-first sorting, and full config info (nickname, create_time, default status, etc.)
  • GET /graphspaces/{graphspace}/graphs/{name}/default — Set a graph as default
  • GET /graphspaces/{graphspace}/graphs/{name}/undefault — Unset a graph as default
  • GET /graphspaces/{graphspace}/graphs/default — Get current user's default graph list
  • PUT /graphspaces/{graphspace}/graphs/{name} — Graph management operations (currently supports update action for nickname updates)
  • POST /graphspaces/{graphspace}/graphs (form-urlencoded) — Hubble frontend form-based graph creation compatibility
  • Auto-fill HStore/PD mode defaults (backend=hstore, serializer=binary, store={name}) during graph creation, and map frontend schema field to schema.init_template

2. GraphSpaceAPI — Default Role Management (GraphSpaceAPI.java)

  • POST /graphspaces/{graphspace}/role — Create default role (supports SPACE/ANALYST/OBSERVER)
  • GET /graphspaces/{graphspace}/role — Check if a user/group has a specified default role
  • DELETE /graphspaces/{graphspace}/role — Delete default role
  • Remove unnecessary PD mode restriction from listProfile endpoint
  • Add @JsonIgnoreProperties(ignoreUnknown = true) to tolerate unknown fields from the frontend

3. ManagerAPI — Role Query (ManagerAPI.java)

  • GET /auth/manager/default — New endpoint for Hubble frontend to query if the current user has a specified default role

4. SchemaTemplateAPI — Schema Template CRUD (New File)

  • GET /graphspaces/{graphspace}/schematemplates — List all schema templates
  • GET /graphspaces/{graphspace}/schematemplates/{name} — Get a specific template
  • POST /graphspaces/{graphspace}/schematemplates — Create template
  • PUT /graphspaces/{graphspace}/schematemplates/{name} — Update template
  • DELETE /graphspaces/{graphspace}/schematemplates/{name} — Delete template

5. Authentication & Authorization Layer (AuthManager.java, StandardAuthManager.java, StandardAuthManagerV2.java, HugeGraphAuthProxy.java)

  • Added 10 new methods to AuthManager interface: setDefaultGraph/unsetDefaultGraph/getDefaultGraph and createDefaultRole/createSpaceDefaultRole/isDefaultRole/deleteDefaultRole, etc.
  • StandardAuthManager and StandardAuthManagerV2 implement the above interfaces using existing HugeGroup/HugeBelong/HugeRole metadata mechanisms
  • HugeGraphAuthProxy.AuthManagerProxy adds corresponding delegate methods to properly forward calls to the underlying authManager

6. Utilities (ConfigUtil.java, GraphManager.java)

  • ConfigUtil.writeConfigToString() — New utility method to serialize graph configuration to string (used by the listProfile endpoint)
  • GraphManager — Added graph management helper methods

Verifying these changes

  • Need tests and can be verified as follows:
    • Verify the full graph management flow via Hubble frontend (create graph, list, profile query)
    • Verify default graph set/unset/query via Hubble frontend
    • Verify Schema template CRUD operations
    • Verify default role (SPACE/ANALYST/OBSERVER) creation, query, and deletion
    • Directly test all new endpoints via REST API

Does this PR potentially affect the following parts?

  • Dependencies (add/update license info & regenerate_known_dependencies.sh)
  • Modify configurations
  • The public API
  • Other affects (typed here)
  • Nope

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

Yeaury added 5 commits April 24, 2026 10:53
- Add listProfile endpoint with default graph sorting and prefix filtering
- Add setDefault/unsetDefault/getDefault endpoints for default graph management
- Add manage(PUT) endpoint for graph nickname update
- Add createByForm for form-urlencoded graph creation compatibility
- Auto-fill HStore/PD defaults (backend/serializer/store) during graph creation
- Add setDefaultRole/checkDefaultRole/deleteDefaultRole in GraphSpaceAPI
- Add checkDefaultRole endpoint in ManagerAPI
- Add default role interfaces in AuthManager
- Implement default role CRUD in StandardAuthManager and StandardAuthManagerV2
- Add stub proxy methods in HugeGraphAuthProxy
- Add new SchemaTemplateAPI with list/get/create/update/delete operations
- Fix package path from api.profile to api.space
- Use HugeGraphAuthProxy.username() instead of authManager.username()
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds/extends HugeGraph Server REST endpoints and auth-layer capabilities needed by the Hubble 2.0 frontend, focusing on graph profile listing, default graph selection, default role management, and schema template CRUD within graphspaces.

Changes:

  • Added graph profile listing + default-graph set/unset/query APIs, plus graph create compatibility tweaks.
  • Implemented default-graph/default-role persistence methods in the AuthManager interface and its implementations/proxies.
  • Introduced schema template CRUD API and corresponding GraphManager helpers.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/util/ConfigUtil.java Adds config-to-string helper used by graph profile listing.
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/AuthManager.java Extends auth interface for default graph/role operations.
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManager.java Implements new default graph/role methods (non-PD auth manager).
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java Implements new default graph/role methods (PD-mode auth manager).
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java Proxies/delegates new AuthManager methods.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java Adds schema template management helpers.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java Adds graph profile listing, default graph APIs, manage/update behavior, and create defaults.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java Adds default role management endpoints and JSON tolerance.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java Adds endpoint to query whether current user has a default role.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java New schema template CRUD endpoint implementation.
Comments suppressed due to low confidence (1)

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java:443

  • configs is only validated as non-null when clone_graph_name is empty. If clone_graph_name is provided and the request body is omitted/empty, configs can be null and convConfig(configs) will throw a NullPointerException in the clone branch. Consider defaulting configs to an empty map (or making convConfig() null-safe) before using it for cloning.
        // Check required parameters for creating graph
        if (StringUtils.isEmpty(clone)) {
            // Only check required parameters when creating new graph, not when cloning
            E.checkArgument(configs != null, "Config parameters cannot be null");
            // Auto-fill defaults for PD/HStore mode when not provided
            configs.putIfAbsent("backend", "hstore");
            configs.putIfAbsent("serializer", "binary");
            configs.putIfAbsent("store", name);
            // Map frontend 'schema' field to backend config key
            Object schema = configs.remove("schema");
            if (schema != null && !schema.toString().isEmpty()) {
                configs.put("schema.init_template", schema.toString());
            }
        }

        String creator = HugeGraphAuthProxy.username();

        if (StringUtils.isNotEmpty(clone)) {
            // Clone from existing graph
            LOG.debug("Clone graph '{}' to '{}' in graph space '{}'", clone, name, graphSpace);
            graph = manager.cloneGraph(graphSpace, clone, name, convConfig(configs));
        } else {
            // Create new graph
            graph = manager.createGraph(graphSpace, name, creator,
                                        convConfig(configs), true);
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…#3008)

- fix: use @POST/@delete for setDefault/unsetDefault (REST semantics)
- fix: add null/empty validation before role field access in GraphSpaceAPI
  to prevent NPE in setDefaultRole/checkDefaultRole/deleteDefaultRole
- fix: change isPrefix to private static and guard nickname null in
  GraphSpaceAPI and GraphsAPI
- fix: ConfigUtil.writeConfigToString always returns JSON regardless
  of whether config was loaded from file, fixing listProfile endpoint
- fix: add @RolesAllowed annotations to SchemaTemplateAPI endpoints
- fix: use ForbiddenException (403) instead of HugeException (400)
  for authorization failures in SchemaTemplateAPI and GraphSpaceAPI
- fix: correct LOG placeholder count in SchemaTemplateAPI.delete
- fix: use HugeException ('%s') format instead of SLF4J '{}' format
- fix: replace com.alipay StringUtils with commons-lang3 in ManagerAPI
- fix: add @consumes and checkUpdate() validation to SchemaTemplate.update
- fix: add ensurePdModeEnabled guard to ManagerAPI.checkDefaultRole
- fix: guard configs null access in GraphsAPI.create clone branch
@imbajin
Copy link
Copy Markdown
Member

imbajin commented Apr 25, 2026

新增 API 单机版(非 PD)兼容性对照

目标:社区默认的 RocksDB 单机版通过 graphspace=DEFAULT 使用 GraphsAPI,应支持图的增删改查和基础权限管理。GraphSpaceAPI、ManagerAPI、SchemaTemplateAPI 属于 PD 专属功能,不需要兼容。

需要兼容单机版的端点(GraphsAPI)

API 端点 当前状态 问题说明 需要的改动
GET .../graphs/profile ⚠️ ConfigUtil.writeConfigToString 序列化全部配置项,可能泄露 password/token 等敏感字段 白名单过滤或排除敏感 key
PUT .../graphs/{name} (manage/update nickname) 1. isExistedGraphNickname() 调用 metaManager.graphConfigs() — 非 PD 下 MetaManager 未初始化,NPE;2. exist.nickname() 只改内存,缺少持久化调用 1. 加 isPDEnabled() 分支,非 PD 下用内存图列表做去重;2. 补充持久化调用
POST .../graphs (create JSON) 无条件执行 configs.putIfAbsent("backend", "hstore"),单机版默认 RocksDB 会被覆盖为 hstore 导致创建失败 if (manager.isPDEnabled()) 包裹 hstore/binary 默认值填充
POST .../graphs/{name} (createByText) 委托 create() 方法,同上 同上(修复 create 即可)
POST .../graphs/{name}/default (设置默认图) StandardAuthManager 已实现
DELETE .../graphs/{name}/default (取消默认图) StandardAuthManager 已实现
GET .../graphs/default (查询默认图) StandardAuthManager 已实现

PD 专属端点(无需兼容单机版)

API 端点 归属 说明
POST/GET/DELETE .../graphspaces/{gs}/role GraphSpaceAPI 图空间级角色管理,PD-only by design
GET /auth/manager/default ManagerAPI 已有 ensurePdModeEnabled() 门禁
GET/POST/PUT/DELETE .../schematemplates[/{name}] SchemaTemplateAPI 全部走 metaManager,PD-only

总结:7 个 GraphsAPI 端点需要兼容单机版。其中 3 个需要代码改动(manage 需加 isPDEnabled() 分支 + 持久化修复,create/createByText 需条件化 hstore 默认值),1 个需过滤敏感配置,3 个已可正常工作。

Yeaury added 2 commits April 26, 2026 16:16
… mode

## Background

Hubble 2.0 previously relied exclusively on PD mode (distributed HStore
backend). This PR makes the server-side APIs fully compatible with the
community default: single-node RocksDB without PD/HStore, so that Hubble
remains functional out of the box for all deployment modes.

## Core bug fixes

### GraphsAPI
- Fix `create()`: `backend=hstore` / `serializer=binary` defaults are now
  only injected when `manager.isPDEnabled()` is true, preventing graph
  creation failures on standalone RocksDB deployments.
- Fix `manage()`: replace `exist.nickname(nickname)` (in-memory only) with
  `manager.updateGraphNickname()`, which persists the change to PD meta
  storage in distributed mode and gracefully falls back to in-memory update
  in standalone mode.
- Fix `manage()`: relax `actionMap.size() == 2` validation to
  `containsKey(GRAPH_ACTION)`, so extra fields from the frontend no longer
  cause spurious 400 errors.
- Guard `getDefaultGraph()`, `setDefault()`, `unsetDefault()`, and
  `getDefault()` with `isPDEnabled()` checks; return empty results in
  standalone mode instead of throwing NPE.
- Fix `listProfile()`: guard `getDefaultGraph()` call with `isPDEnabled()`;
  add null-safe fallback for `gs.nickname()` in non-PD mode.

### GraphManager
- `isExistedGraphNickname()`: add non-PD branch that scans in-memory graphs
  instead of accessing the uninitialized `metaManager`, preventing NPE in
  standalone mode.
- New `updateGraphNickname()`: updates in-memory graph instance first, then
  persists nickname to `metaManager` only in PD mode.

### ConfigUtil
- `writeConfigToString()`: always serializes config to JSON (previously
  could emit raw properties format), fixing `listProfile` deserialization.
- New `isSensitiveKey()`: filters keys containing `password`, `secret`,
  `token`, `credential`, `private_key`, or `auth.key` from the serialized
  output to prevent credential leakage through the API.

### ManagerAPI
- Add `ensurePdModeEnabled()` guard to all PD-specific endpoints
  (`createManager`, `deleteManager`, `list`, `checkRole`, `getRolesInGs`,
  `checkDefaultRole`).
- Wrap `HugeDefaultRole.valueOf()` in try-catch to return HTTP 400 instead
  of HTTP 500 when an invalid role string is supplied.

### SchemaTemplateAPI
- Fix incorrect `HugeException` import; replace with `ForbiddenException`
  for proper HTTP 403 semantics.
- Add missing `@RolesAllowed` annotations and implement `checkUpdate()`
  validation.

### StandardAuthManager
- Implement `setDefaultGraph` / `getDefaultGraph` / `unsetDefaultGraph`
  using marker-group pattern (HugeGroup + HugeBelong) for persistence.
- Implement `createDefaultRole` / `createSpaceDefaultRole` /
  `isDefaultRole` / `deleteDefaultRole` with the same marker-group
  mechanism.
- Add detailed design-note Javadoc explaining the workaround, its
  limitations, and the non-PD degradation path.

## Code quality improvements

- Extract shared `isPrefix(Map, String)` helper and `DATE_FORMATTER`
  constant into the `API` base class, eliminating ~30 lines of duplicated
  code across `GraphsAPI` and `GraphSpaceAPI`.
- Replace non-thread-safe `SimpleDateFormat` (constructed per-request) with
  a single static `DateTimeFormatter` (immutable, thread-safe).
- Fix 12-hour clock format `hh` → 24-hour `HH` in `GraphSpaceAPI`.
@Yeaury Yeaury marked this pull request as ready for review April 26, 2026 09:47
@dosubot dosubot Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Apr 26, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 10 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Yeaury added 3 commits May 5, 2026 15:18
Add the private static isPrefix() method that was accidentally removed
in a previous commit. This method is used by listProfile() to filter
graphs by name or nickname prefix, with proper null handling for
nickname field.

This fixes a compilation error where isPrefix() was called but not
defined.
Check if belong record exists before attempting to delete in
unsetDefaultGraph() to prevent throwing exceptions when unsetting
an already-unset default graph. This makes the API idempotent and
prevents 500 errors when clients call unset multiple times.

The fix aligns StandardAuthManagerV2 with StandardAuthManager, which
already implements this idempotent behavior.
- SchemaTemplateAPI: add ensurePdModeEnabled() guard to all endpoints
  to prevent NPE when MetaManager is not initialized in standalone mode
- SchemaTemplateAPI: add null body check in create() and update() to
  return 400 instead of NPE/500 on missing request body
- GraphsAPI.listProfile: format default_update_time consistently with
  create_time using DATE_FORMATTER instead of raw Date serialization
- GraphsAPI.manage: guard value.getClass() against null action value
  to prevent NPE when building the validation error message
- GraphSpaceAPI.deleteDefaultRole: wrap HugeDefaultRole.valueOf() in
  try/catch to return 400 for invalid role values instead of 500
- GraphManager: add isPDEnabled() guard to all schema-template methods
  to prevent NPE when MetaManager is not available in standalone mode
- GraphManager.updateGraphNickname: propagate PD persistence failure to
  caller with in-memory rollback, preventing silent state inconsistency
Yeaury added 2 commits May 5, 2026 17:34
- GraphsAPI: remove duplicate private isPrefix() that shadowed the
  protected version in API base class, causing compilation failure
- GraphsAPI: fix NPE in manage() when 'update' value is null by
  guarding value.getClass() in both action and update validation
- GraphsAPI/GraphSpaceAPI: replace org.apache.commons.lang.StringUtils
  with commons-lang3 and remove org.apache.logging.log4j.util.Strings
  import (replaced with StringUtils equivalents) to fix potential
  classpath issues
- GraphManager.updateGraphNickname: restore old nickname (not null)
  on PD persistence failure to avoid a third inconsistent state
- StandardAuthManagerV2.setDefaultGraph: check existBelong() before
  createBelong() to make the operation idempotent; repeated POST calls
  for the same user/graph no longer throw HugeException
- StandardAuthManagerV2.createDefaultRole: same idempotent guard via
  existBelong() check before createBelong()
- ManagerAPI.checkDefaultRole: use @PathParam graphspace instead of
  @QueryParam to match the mounted path contract
StandardAuthManager (standalone RocksDB) fully implements setDefaultGraph/
unsetDefaultGraph/getDefaultGraph via HugeGroup+HugeBelong mechanism and
does not require PD. The non-PD early-return guards in setDefault, unsetDefault,
getDefault and listProfile were silently discarding user preferences, making
the API a no-op in standalone mode.

Remove the isPDEnabled() short-circuits and route all default graph operations
directly through authManager, which dispatches correctly to either
StandardAuthManager or StandardAuthManagerV2 depending on the deployment mode.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 8 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Yeaury added 3 commits May 5, 2026 23:54
…nding

- Fix SchemaTemplateAPI logger using wrong class (RestServer -> SchemaTemplateAPI)
- Fix StandardAuthManager fabricated belong ID: replace string-constructed IDs
  with actual graph traversal (findBelongBinding) to correctly detect/delete
  existing HugeBelong edges, making setDefaultGraph/unsetDefaultGraph/
  createDefaultRole idempotent
- Fix HugeUser.initSchemaIfNeeded: add incremental schema upgrade so existing
  deployments get the user_nickname property key without full schema recreation
…rors

Server-side fixes:

- GraphsAPI.setDefault/unsetDefault/getDefault and GraphSpaceAPI role endpoints now handle missing authManager (standalone mode) gracefully instead of throwing unhandled IllegalStateException
- STANDALONE_ERROR made protected in API.java for subclass access

New tests (29 total, all passing on rocksdb):

- GraphsApiStandaloneTest: profile, default graph, nickname update
- SchemaTemplateApiTest: all CRUD endpoints standalone error tests
- DefaultRoleApiStandaloneTest: role endpoints standalone error tests
@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.25%. Comparing base (9126c80) to head (26a2c42).
⚠️ Report is 19 commits behind head on master.

Additional details and impacted files
@@              Coverage Diff              @@
##             master    #3008       +/-   ##
=============================================
+ Coverage     35.57%   93.25%   +57.68%     
+ Complexity      333       65      -268     
=============================================
  Files           801        9      -792     
  Lines         67592      267    -67325     
  Branches       8790       22     -8768     
=============================================
- Hits          24045      249    -23796     
+ Misses        40985        8    -40977     
+ Partials       2562       10     -2552     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 19 changed files in this pull request and generated 7 comments.

Comment on lines +230 to +278
@POST
@Timed
@Path("{name}/default")
@Produces(APPLICATION_JSON_WITH_CHARSET)
@RolesAllowed({"space_member", "$owner=$name"})
public Map<String, Object> setDefault(@Context GraphManager manager,
@Parameter(description = "The graph space name")
@PathParam("graphspace") String graphSpace,
@Parameter(description = "The graph name")
@PathParam("name") String name) {
LOG.debug("Set default graph '{}' in graph space '{}'", name, graphSpace);
E.checkArgument(manager.graph(graphSpace, name) != null,
"Graph '%s/%s' does not exist", graphSpace, name);
String user = HugeGraphAuthProxy.username();
AuthManager authManager;
try {
authManager = manager.authManager();
} catch (IllegalStateException e) {
throw new HugeException(STANDALONE_ERROR);
}
authManager.setDefaultGraph(graphSpace, name, user);
Map<String, Date> defaults = authManager.getDefaultGraph(graphSpace, user);
return ImmutableMap.of("default_graph", defaults.keySet());
}

@DELETE
@Timed
@Path("{name}/default")
@Produces(APPLICATION_JSON_WITH_CHARSET)
@RolesAllowed({"space_member", "$owner=$name"})
public Map<String, Object> unsetDefault(@Context GraphManager manager,
@Parameter(description = "The graph space name")
@PathParam("graphspace") String graphSpace,
@Parameter(description = "The graph name")
@PathParam("name") String name) {
LOG.debug("Unset default graph '{}' in graph space '{}'", name, graphSpace);
E.checkArgument(manager.graph(graphSpace, name) != null,
"Graph '%s/%s' does not exist", graphSpace, name);
String user = HugeGraphAuthProxy.username();
AuthManager authManager;
try {
authManager = manager.authManager();
} catch (IllegalStateException e) {
throw new HugeException(STANDALONE_ERROR);
}
authManager.unsetDefaultGraph(graphSpace, name, user);
Map<String, Date> defaults = authManager.getDefaultGraph(graphSpace, user);
return ImmutableMap.of("default_graph", defaults.keySet());
}
Comment on lines +265 to +303
@GET
@Timed
@Path("default")
@Consumes(APPLICATION_JSON)
public String checkDefaultRole(@Context GraphManager manager,
@PathParam("graphspace") String graphSpace,
@QueryParam("role") String role,
@QueryParam("graph") String graph) {
LOG.debug("check if current user is default role: {} {} {}",
role, graphSpace, graph);
ensurePdModeEnabled(manager);
AuthManager authManager = manager.authManager();
String user = HugeGraphAuthProxy.username();

E.checkArgument(StringUtils.isNotEmpty(role) &&
StringUtils.isNotEmpty(graphSpace),
"Must pass graphspace and role params");

HugeDefaultRole defaultRole;
try {
defaultRole = HugeDefaultRole.valueOf(role.toUpperCase());
} catch (IllegalArgumentException e) {
E.checkArgument(false, "Invalid role value '%s'", role);
defaultRole = null; // unreachable, satisfies compiler
}
boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER);
E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph),
"Must set a graph for observer");

boolean result;
if (hasGraph) {
result = authManager.isDefaultRole(graphSpace, graph, user,
defaultRole);
} else {
result = authManager.isDefaultRole(graphSpace, user,
defaultRole);
}
return manager.serializer().writeMap(ImmutableMap.of("check", result));
}
Copy link
Copy Markdown
Member

@imbajin imbajin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a few remaining issues that can affect startup cleanup, default-role correctness, and API error semantics. I also added +1 reactions to existing Copilot comments for duplicate findings instead of reposting them.

@imbajin
Copy link
Copy Markdown
Member

imbajin commented May 21, 2026

Follow-up TODOs that are not necessarily blockers for this PR, but are worth tracking separately if they are not handled here:

  1. Consider using one documented marker convention for default graph/default role metadata. V1 uses a reserved ~default_* prefix while V2 uses the _DEFAULT_SETTER_ROLE suffix; the suffix form can collide with user-defined role names that happen to end with that marker.
  2. Consider splitting the default graph/default role storage workaround into a follow-up issue. The marker-group approach is now documented, but it still leaks implementation details into auth entities and may deserve a dedicated storage model later.
  3. Consider replacing role.equals(HugeDefaultRole.OBSERVER) checks with role.isGraphRole() so future graph-scoped default roles do not need to touch all API branches again.
  4. JsonDefaultRole implements Checkable, but checkCreate() and checkUpdate() are empty while validation is inline in the API method. Either move validation into the DTO or drop the interface to avoid a misleading contract.
  5. Rename @PathParam("graphspace") String name to graphSpace in the default-role methods for readability and consistency with the surrounding APIs.
  6. GraphsAPI.listProfile() still calls manager.graphSpace(graphSpace) twice and the diff has a few trailing-whitespace lines. These are low-risk cleanups if another revision is already planned.
  7. The response ordering in GraphsAPI.listProfile() appears to put default profiles first, then other profiles. If the frontend depends on this, please document or explicitly sort the response.

Some broader test gaps are already covered by existing review comments, so I am not repeating them here. If these TODOs are out of scope for this PR, they can be split into separate tracking issues.

Copy link
Copy Markdown
Member

@imbajin imbajin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review after the latest update: these are remaining current-PR issues that look small enough to fix directly, while already-covered findings are intentionally not duplicated.

Yeaury and others added 2 commits May 21, 2026 19:19
- Restore SecurityManager and clean up prepared GremlinServer on startup failures
- Resolve default-role owner type detection for group bindings
- Validate graphspace and observer graph ownership in default-role APIs
- Return 4xx for malformed graph manage requests instead of server errors
- Invalidate user cache after default graph/default role mutations
- Make V2 default-role deletion idempotent for repeated requests
- Validate graph create/clone config values and reject null or non-scalar input
- Avoid rolling back local nickname state after successful PD persistence
- Remove misleading @consumes annotations from entity-less default-role endpoints
- Add standalone tests for malformed graph API requests
- Add hstore-gated graph default lifecycle coverage for graphspace graphs

- Add default role lifecycle coverage through the graphspace role API

- Add schema template lifecycle coverage in PD/HStore mode

- Clean trailing whitespace in GraphsAPI
@imbajin
Copy link
Copy Markdown
Member

imbajin commented May 21, 2026

Follow-up TODO for CI coverage:

Add a real distributed-mode CI job that boots PD + Store and runs the hstore-backed graphspace API tests against that environment. The tests added in this PR compile and are gated to backend=hstore, but they still rely on an external PD/Store runtime to prove the full happy path.

Suggested scope for a separate issue:

  1. Start a minimal PD + Store + Server stack in CI.
  2. Run the hstore/PD graphspace API suite, including default graph, default role, and schema template lifecycle coverage.
  3. Keep the existing standalone/RocksDB compatibility tests separate so both modes remain visible.

This is not necessarily a blocker for the current PR, but it is the right follow-up to prevent these Hubble-compatible APIs from only being validated at compile time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Changes of API feature New feature size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

3 participants